Skip to content

Add Authentication & User history#34

Merged
DerouineauNicolas merged 117 commits intomasterfrom
tokenauth
Jun 10, 2020
Merged

Add Authentication & User history#34
DerouineauNicolas merged 117 commits intomasterfrom
tokenauth

Conversation

@DerouineauNicolas
Copy link
Copy Markdown
Member

This PR aims to adress issue #4 and #5.

The follwing features are added:

  • adds token-auth authentification to DRF backend

  • add login/subcribe pages in the frontend

  • push informations from front to back when a authenticated user watches a video (videoid and current playing time in the media)

  • allow authenticated users to get their viewing history in the frontend.

Moreover, several improvements have been done on the frontend side (code refactoring and UI).

A cross check for existing files in the database is also added when updating database:

docker-compose -f docker-compose-prod.yml run --rm web python3 manage.py updatedb

@xavierfav xavierfav changed the title Add Authentication/User history Add Authentication & User history Jun 9, 2020
Copy link
Copy Markdown
Member

@xavierfav xavierfav left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this PR is very cool.
I pointed several weird python condition testing in the code.

For the front, I think we still need to organize it a bit better, but we can leave that for future work.
The authentication is a bit messy, and is sometimes using the API client, and other times creating a new Axios client which is a bit weird, inconsistent, and less effective than always using the same client.

return return_value


def populate_db_from_remote_server(remotePath, ListOfVideos):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is empty

"""
cmd = ["ffmpeg", "-n", "-sub_charenc", "UTF-8", "-i", input_file, output_file]
if(os.path.isfile(output_file) == False):
if(os.path.isfile(output_file) is False):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When doing conditions on boolean values in Python, we usually would do:

if not os.path.isfile(output_file):

Have a look here for instance.

cmd = ["ffmpeg", "-n", "-sub_charenc", "UTF-8", "-i", input_file,"-map", "0:s:0", output_file]
if(os.path.isfile(output_file) == False):
cmd = ["ffmpeg", "-n", "-sub_charenc", "UTF-8", "-i", input_file, "-map", "0:s:0", output_file]
if(os.path.isfile(output_file) is False):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above

"-codec", "copy", "-movflags", "+faststart", output_file]

if(os.path.isfile(output_file) == False):
if(os.path.isfile(output_file) is False):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above

cmd = ["ffmpeg", "-ss", str(duration/2.0), "-i", input_file, "-an", "-vf", "scale=320:-1",
"-vframes", "1", output_file]
if(os.path.isfile(output_file) == False):
if(os.path.isfile(output_file) is False):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above

Comment thread backend/StreamServerApp/subtitles.py Outdated
""" # init cache for subtitles database query and stuff.
"""
if(os.path.isfile('cachefile.dbm.db') == False):
if(os.path.isfile('cachefile.dbm.db') is False):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above

video_path, Language(langage))
webvtt_en_fullpath = os.path.splitext(srt_fullpath)[0]+'.vtt'
if(os.path.isfile(webvtt_en_fullpath) == True):
if(os.path.isfile(webvtt_en_fullpath) is True):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if os.path.isfile(webvtt_en_fullpath):

if(os.path.isfile(webvtt_en_fullpath) is True):
#return subtitles path even if subtitles are already downloaded/converted
return webvtt_en_fullpath
if(os.path.isfile(srt_fullpath)):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above

Comment thread backend/StreamServerApp/urls.py Outdated
router.register(r'videos', videos.VideoViewSet, basename='videos')
router.register(r'series', videos.SeriesViewSet, basename='series')
router.register(r'movies', videos.MoviesViewSet, basename='movies')
# router.register(r'history', accounts.HistoryViewSet, basename='history')
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

left commented code here

Comment thread backend/StreamingServer/settings.py Outdated
if (VERBOSE_OUTPUT == True):
customstdout=subprocess.PIPE
customstderr=subprocess.PIPE
if (VERBOSE_OUTPUT is True):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if VERBOSE_OUTPUT:

Comment thread frontend/src/components/login/login.js Outdated
const { setAuthTokens } = useAuth();

function postLogin() {
const http = axios.create({
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we create a new axios client here?

Comment thread frontend/src/components/login/signup.js Outdated
const { setAuthTokens } = useAuth();

function postSignup() {
const http = axios.create({
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above

Comment thread frontend/src/components/login/signup.js Outdated
baseURL: process.env.REACT_APP_DJANGO_API,
responseType: 'json',
});
http.post("/rest-auth/registration/", {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here we use the API client, but it would be nice not to hardcode any API url in the front code, and organize it like we started in the API client file.

Copy link
Copy Markdown
Member

@xavierfav xavierfav left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, thanks for the changes @DerouineauNicolas.
It's still not perfect for the authentication feature in front (e.g., harcoded path and no client method for login/logout/singup), but I think it is enough for now!
Good job!

@DerouineauNicolas DerouineauNicolas merged commit 714a911 into master Jun 10, 2020
@DerouineauNicolas DerouineauNicolas deleted the tokenauth branch June 11, 2020 08:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants